Skipping borrowck because of trivial const#149468
Skipping borrowck because of trivial const#149468chenyukang wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
I'm okay with this change, but I would like @lcnr's opinion on whether this sort of whack-a-mole approach is sustainable. (I don't want to have an ever-growing number of |
|
What are the requirements for something to be a trivial const, are they written down somewhere? E.g. is I personally feel quite... unhappy about this change. @BoxyUwU has been changing lowering of some constants for const generics to never actually get lowered to HIR expression but instead eagerly convert them into type system constants. That means we don't have a body for them and don't even try to call any of the Having both of these impls seems fairly bad to me. It seems like ideally there should only be one way to get constants without corresponding MIR body. I don't have the time to engage with that area myself right now :x I think we should not have both systems and ideally you'd chat with @BoxyUwU to figure out whether you can unify the two somehow |
|
Well, that's why I asked. I agree that Boxy's approach seems better. If you wanted to know the semantics I implemented, they're all in here: https://github.com/rust-lang/rust/blob/2fb805367dd3012ce5c50865d03c86e70bf10f0f/compiler/rustc_mir_transform/src/trivial_const.rs |
so this ends up as a trivial const? we never check that the type of the const is not an opaque. This code would start to fail/misbehave with this PR because MIR borrowck doesn't return the defining use anymore, would it not (at least with the new solver)? |
|
Would definitely be interested in chatting about things here 🤔 @saethlin if you wanna reach out on zulip at some point that'd be cool :) |
|
Checking for progress. Did a conversation happen about this? What's the current status? thanks for an update :) |
r? @saethlin
the assertion is added in PR: #148040
I think we also need to skip trvial const in
mir_borrowck.